Skip to content

fix(knn): reapply sort after rescoring distances#4588

Open
sanikolaev wants to merge 1 commit into
masterfrom
fix/knn-rescore-stable-order
Open

fix(knn): reapply sort after rescoring distances#4588
sanikolaev wants to merge 1 commit into
masterfrom
fix/knn-rescore-stable-order

Conversation

@sanikolaev
Copy link
Copy Markdown
Collaborator

Related issue #4320

@sanikolaev sanikolaev force-pushed the fix/knn-rescore-stable-order branch from 9536712 to 12501e9 Compare May 21, 2026 04:21
@manticoresoftware manticoresoftware deleted a comment from github-actions Bot May 21, 2026
@sanikolaev sanikolaev force-pushed the fix/knn-rescore-stable-order branch from 12501e9 to 262fdc0 Compare May 21, 2026 05:12
@sanikolaev sanikolaev changed the title fix(knn): preserve prior order for equal rescored distances fix(knn): reapply sort after rescoring distances May 21, 2026
@sanikolaev sanikolaev force-pushed the fix/knn-rescore-stable-order branch from 262fdc0 to 6b7bc79 Compare May 21, 2026 05:32
Related issue #4320

KNN rescore now copies exact distances into the original knn_dist() sort key and reruns the original comparator, so explicit ORDER BY tie-breakers are respected for equal rescored distances.
@sanikolaev sanikolaev force-pushed the fix/knn-rescore-stable-order branch from 6b7bc79 to d9e3698 Compare May 21, 2026 08:01
@manticoresoftware manticoresoftware deleted a comment from github-actions Bot May 21, 2026
@manticoresoftware manticoresoftware deleted a comment from github-actions Bot May 21, 2026
@sanikolaev
Copy link
Copy Markdown
Collaborator Author

@glookka, previously there was another attempt to fix this issue, and you wrote.

I looked through the code — the problem is that MatchSortRescore_fn does not copy the full sorting logic (across all attributes) from the original sorter. The correct fix is to apply the same sorting criteria. The suggested fix is incorrect.

The change in this PR fixes it according to your comment. Pls review.

@sanikolaev sanikolaev requested a review from glookka May 21, 2026 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant